test(routing): pin queryParameter, which a report claimed was broken - #147
Conversation
A consumer put `?scale=5000` on a route, could not read it back, and filed it
against this package. The evidence was `dusk:get_routes` reporting
`location: /` while the browser sat on `#/?scale=5000`.
That evidence measures something else. Dusk's `location` is
`route.settings.name` off the Navigator (`ext_navigation.dart:442`), which
under `MaterialApp.router` is the declared path pattern, not the resolved URI.
It was always going to read `/` and says nothing about what this router saw.
These six tests settle it, and all six passed on the first run: the router
reads a query parameter correctly, both when it is passed through the
`queryParameters` argument and when it is written inline in the path, which is
the shape `to('/?scale=5000')` produces and the one the report used. So there
is nothing to fix here, and the report is withdrawn.
They stay because the API had no regression test at all, and because the next
person who doubts it should be able to settle it in one command rather than by
reasoning about GoRouter. The fourth one is the interesting case: a later
navigation with no query has to CLEAR the value rather than leave a stale one a
caller would read as current.
Tests only, no `lib/` change, so the post-change sync does not apply and there
is no CHANGELOG entry: nothing about the behaviour moved.
|
Note Kodizm (AI-generated). May contain mistakes; verify before acting. Test-only addition that does what it claims: the six cases pass locally, the accessors genuinely had no prior coverage, and the file follows the conventions of its sibling I verified the "no existing coverage" claim: I also confirmed the Minor
TestsThis is the tests. Six cases covering both No Checks I ran
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…ation Three points from review. One test name read backwards from what it asserts. "survives a navigation that carries no query at all" says the value persists; the body, the inline comment and the pull request description all say it is cleared, which is the behaviour worth pinning. Renamed to say so. The pre-resolution case pumped nothing and took a `tester` it never used. `.claude/rules/tests.md:18` asks for `test()` for pure logic, and the sibling file already does exactly that for its own no-route-yet case. Now a `test()`. `Request.query` and `Request.queryParams` (`request.dart:72` and `:83`) are one-line delegations to the two accessors this file covers, and nothing tested them either. They are the surface a consumer actually reads, and reading through them is what the original report was doing, so the pairing is the part worth having under test rather than the one-liners themselves.
|
Note Kodizm (AI-generated). May contain mistakes; verify before acting. Both minors from the last pass are fixed and the Since TestsSeven cases now: six on the router accessors, one on the Checks I ran
|
1 participant
The report, and why it was wrong
A consumer put
?scale=5000on a route, could not read it back, and filed it here. The evidence wasdusk:get_routesprintinglocation: /while the browser sat onhttp://localhost:3210/#/?scale=5000.That evidence measures something else. Dusk's
locationisroute.settings.nametaken off the Navigator (fluttersdk_dusk,lib/src/extensions/ext_navigation.dart:442), which underMaterialApp.routeris the declared path pattern rather than the resolved URI. It was always going to read/, whatever this router had seen.All six tests below passed on the first run.
MagicRouter.queryParameterreads the query correctly, including from the inline formto('/?scale=5000')that the report used. There is nothing to fix and the report is withdrawn.Why keep the tests
queryParameterandqueryParametershad no regression test anywhere intest/. The next person who doubts them should be able to settle it with one command instead of reasoning about how GoRouter parses a location.Six cases:
queryParametersargumentto('/?scale=5000'))queryParametersexposes every pairqueryParametersis empty rather than null before any route resolvesGates
dart format .: 334 files, 0 changed.dart analyzeon the new file: no issues.flutter test: 1426 passed, 6 of them new.Tests only, no change under
lib/, so the post-change sync does not apply and there is no CHANGELOG entry: nothing about the behaviour moved.